Reuse `FSSpecTarget` Auth Credentials #668

ranchodeluxe · 2024-01-17T15:23:41Z

Addresses #666

Goal was to remove storage options from being passed in WriteCombinedReference b/c we already had access to the target_root being dep injected. But then for all nested fsspec instantiations within WriteCombinedReference we had to figure out how to return the original fsspec options from target_root and pass them along

Companion PR in Runner

pangeo-forge/pangeo-forge-runner#163

Release Steps

Merge Help Reuse FSSpecTarget Auth Credentials pangeo-forge-runner#163 first
Do a pangeo-forge-runner 0.9.3 release
Change this integration tests line in this repo to point to pangeo-forge-runner==0.9.3
Make sure all tests pass
Merge and Profit

pangeo_forge_recipes/storage.py

pangeo_forge_recipes/types.py

pangeo_forge_recipes/writers.py

pangeo_forge_recipes/transforms.py

pangeo_forge_recipes/writers.py

pangeo_forge_recipes/transforms.py

.github/workflows/test-integration.yaml

norlandrhagen · 2024-01-22T18:28:58Z

pangeo_forge_recipes/combiners.py

@@ -65,7 +65,7 @@ class CombineMultiZarrToZarr(beam.CombineFn):
      along a dimension that does not exist in the individual inputs. In this latter
      case, precombining adds the additional dimension to the input so that its
      dimensionality will match that of the accumulator.
-    :param storage_options: Storage options dict to pass to the MultiZarrToZarr
+    :param target_options: Storage options dict to pass to the MultiZarrToZarr


docstring following target_options still reads Storage options.

norlandrhagen · 2024-01-22T18:30:46Z

pangeo_forge_recipes/storage.py

+        elif isinstance(fsspec_protocol, list):
+            return fsspec_protocol[0]
+        else:
+            raise ValueError("could not resolve fsspec protocol from underlying filesystem")


Maybe we could return what fsspec_protocol is in the ValueError if it isn't caught by any of the isinstance checks?

norlandrhagen · 2024-01-22T18:31:02Z

pangeo_forge_recipes/transforms.py

@@ -466,9 +465,6 @@ class WriteReference(beam.PTransform, ZarrWriterMixin):
      will be appended to this prefix to create a full path.
    :param output_file_name: Name to give the output references file
      (``.json`` or ``.parquet`` suffix).
-    :param target_options: Storage options for opening target files


So nice to get rid of all of these

norlandrhagen · 2024-01-22T18:32:20Z

tests/test_end_to_end.py

@@ -46,14 +46,14 @@ def test_xarray_zarr(
            | beam.Create(pattern.items())
            | OpenWithXarray(file_type=pattern.file_type)
            | StoreToZarr(
-                target_root=tmp_target_url,
+                target_root=tmp_target,


Glad this works!

norlandrhagen

Looks great @ranchodeluxe! Nice to remove all of the super nested fsspec stuff being passed around. Left a few minor comments.

ranchodeluxe · 2024-01-25T20:04:06Z

@cisaacstern: gonna merge this since we talked about getting our kerchunk recipes squared away today

ranchodeluxe changed the title ~~hack~~ Do not Resuse FSSpecTarget Auth Credentials Jan 17, 2024

ranchodeluxe marked this pull request as draft January 17, 2024 15:24

ranchodeluxe changed the title ~~Do not Resuse FSSpecTarget Auth Credentials~~ Do Not Resuse FSSpecTarget Auth Credentials Jan 17, 2024

ranchodeluxe mentioned this pull request Jan 17, 2024

Add Config to Pass Around Injected Args #652

Open